Skip to content

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Oct 6, 2025

The validation for when to colorize or not was happening in a loop. Instead, it is now done once up front instead.

The validation for when to colorize or not was happening in a loop.
Instead, it is now done once up front instead.
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Oct 6, 2025
@marco-ippolito
Copy link
Member

marco-ippolito commented Oct 6, 2025

I think the side effect is that the input is no longer validated when colorize is disabled

@BridgeAR
Copy link
Member Author

BridgeAR commented Oct 6, 2025

@marco-ippolito yes, that is correct. It is the question if we need to validate the colors if they are not used.

@marco-ippolito
Copy link
Member

@marco-ippolito yes, that is correct. It is the question if we need to validate the colors if they are not used.

I lean more on the yes we should always validate the input (that's why when I previously worked on this api I left the check in the loop) but I really dont have a strong opinion, just wanted to highlight this change

Copy link

codecov bot commented Oct 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.53%. Comparing base (7c85aa5) to head (929c3c3).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #60133      +/-   ##
==========================================
+ Coverage   88.52%   88.53%   +0.01%     
==========================================
  Files         703      703              
  Lines      207825   207807      -18     
  Branches    40003    40008       +5     
==========================================
+ Hits       183976   183992      +16     
+ Misses      15862    15815      -47     
- Partials     7987     8000      +13     
Files with missing lines Coverage Δ
lib/util.js 97.96% <100.00%> (-0.01%) ⬇️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants